fix(react-native): Enhance fragment detection for indirect references #767

antonis · 2025-07-18T13:57:49Z

Fixes getsentry/sentry-react-native#4902

AbhiPrasad

overall approach seem good to me. Anecdotally traversing imports usually is not that expensive, but it might be nice to do a sanity check benchmark.

AbhiPrasad · 2025-07-21T08:36:31Z

packages/babel-plugin-component-annotate/src/index.ts

+      Program: {
+        enter(path, state) {
+          const fragmentContext = collectFragmentContext(path);
+          state['sentryFragmentContext'] = fragmentContext;


We should update AnnotationPluginPass to strongly type sentryFragmentContext.

Good idea 👍 Updated with b44fcc8

AbhiPrasad · 2025-07-21T08:44:48Z

packages/babel-plugin-component-annotate/src/index.ts

@@ -151,7 +166,8 @@ function functionBodyPushAttributes(
  componentName: string,
  sourceFileName: string | undefined,
  attributeNames: string[],
-  ignoredComponents: string[]
+  ignoredComponents: string[],
+  fragmentContext?: FragmentContext


Might be time to convert this to take an object instead of plain args (and also add a jsdoc to document)

Updated with d728ba5

Lms24

Sounds reasonable to me -- thanks for adding all the tests!

Besides Abhi's feedback, could you take a look at the integration test? Perhaps we can add a fragment or two there as well (but happy to leave this up to you. If it's too much of a hassle that's fine as well)

antonis · 2025-07-21T13:59:03Z

Thank you both for the reviews and feedback. I really appreciate this 🙇
I'll iterate on it and mark the PR as ready for review 🙏

antonis · 2025-08-07T08:01:48Z

Thanks again for your feedback 🙇

This is now ready for another pass.

Anecdotally traversing imports usually is not that expensive, but it might be nice to do a sanity check benchmark.

@AbhiPrasad I've added some performance tests with 4063095

Besides Abhi's feedback, could you take a look at the integration test? Perhaps we can add a fragment or two there as well (but happy to leave this up to you. If it's too much of a hassle that's fine as well)

@Lms24 I think this is a good idea 👍 I'm facing some issues running the integration tests (unmodified) localy (see log). I'll investigate this further and iterate on this or a follow up PR. Any feedback is welcome 🙏

log

~/git/sentry-javascript-bundler-plugins$ yarn test:integration                      

   ✔    6/6 dependent project tasks succeeded [5 read from cache]

   Hint: you can run the command with --verbose to see the full dependent project outputs


> nx run @sentry-internal/integration-tests:test

...

fixtures/bundle-size-optimizations/out/vite/bundle1.js   0.37 KiB / gzip: 0.23 KiB
fixtures/bundle-size-optimizations/out/vite/bundle1.js.map 0.36 KiB
fixtures/bundle-size-optimizations/out/vite/bundle2.js   0.45 KiB / gzip: 0.28 KiB
fixtures/bundle-size-optimizations/out/vite/bundle2.js.map 0.56 KiB
[sentry-esbuild-plugin] Warning: The component name annotate plugin is currently not supported by '@sentry/esbuild-plugin'
✘ [ERROR] Could not resolve "react-dom/server"

    fixtures/component-name-annotate/input/app.jsx:1:31:
      1 │ import { renderToString } from "react-dom/server";
        ╵                                ~~~~~~~~~~~~~~~~~~

  The Yarn Plug'n'Play manifest forbids importing "react-dom" here because it's not listed as a dependency of this package:

    ../../../../.pnp.cjs:65:33:
      65 │           "packageDependencies": [\
         ╵                                  ~~

  You can mark the path "react-dom/server" as external to exclude it from the bundle, which will remove this error and leave the unresolved path in the bundle.

✘ [ERROR] Could not resolve "react/jsx-runtime"

    fixtures/component-name-annotate/input/app.jsx:5:9:
      5 │   return <ComponentA />;
        ╵          ^

  The Yarn Plug'n'Play manifest forbids importing "react" here because it's not listed as a dependency of this package:

    ../../../../.pnp.cjs:65:33:
      65 │           "packageDependencies": [\
         ╵                                  ~~

  You can mark the path "react/jsx-runtime" as external to exclude it from the bundle, which will remove this error and leave the unresolved path in the bundle.

✘ [ERROR] Could not resolve "react/jsx-runtime"

    fixtures/component-name-annotate/input/component-a.jsx:2:9:
      2 │   return <span>Component A</span>;
        ╵          ^

  The Yarn Plug'n'Play manifest forbids importing "react" here because it's not listed as a dependency of this package:

    ../../../../.pnp.cjs:65:33:
      65 │           "packageDependencies": [\
         ╵                                  ~~

  You can mark the path "react/jsx-runtime" as external to exclude it from the bundle, which will remove this error and leave the unresolved path in the bundle.

[sentry-esbuild-plugin] Warning: No auth token provided. Will not create release. Please set the `authToken` option. You can find information on how to generate a Sentry auth token here: https://docs.sentry.io/api/auth/
[sentry-esbuild-plugin] Warning: No auth token provided. Will not upload source maps. Please set the `authToken` option. You can find information on how to generate a Sentry auth token here: https://docs.sentry.io/api/auth/
/Users/antonis/git/sentry-javascript-bundler-plugins/node_modules/esbuild019/lib/main.js:1650
  let error = new Error(text);
              ^
Error: Build failed with 3 errors:
fixtures/component-name-annotate/input/app.jsx:1:31: ERROR: Could not resolve "react-dom/server"
fixtures/component-name-annotate/input/app.jsx:5:9: ERROR: Could not resolve "react/jsx-runtime"
fixtures/component-name-annotate/input/component-a.jsx:2:9: ERROR: Could not resolve "react/jsx-runtime"
    at failureErrorWithLog (/Users/antonis/git/sentry-javascript-bundler-plugins/node_modules/esbuild019/lib/main.js:1650:15)
    at /Users/antonis/git/sentry-javascript-bundler-plugins/node_modules/esbuild019/lib/main.js:1059:25
    at /Users/antonis/git/sentry-javascript-bundler-plugins/node_modules/esbuild019/lib/main.js:1526:9
    at processTicksAndRejections (node:internal/process/task_queues:95:5) {
  errors: [Getter/Setter],
  warnings: [Getter/Setter]
}
Error: Command failed: yarn ts-node --transpileOnly /Users/antonis/git/sentry-javascript-bundler-plugins/packages/integration-tests/fixtures/component-name-annotate/setup.ts
    at checkExecSyncError (node:child_process:890:11)
    at Object.execSync (node:child_process:962:15)
    at /Users/antonis/git/sentry-javascript-bundler-plugins/packages/integration-tests/scripts/run-fixture-setups.ts:12:18
    at Array.forEach (<anonymous>)
    at Object.<anonymous> (/Users/antonis/git/sentry-javascript-bundler-plugins/packages/integration-tests/scripts/run-fixture-setups.ts:9:14)
    at Module._compile (node:internal/modules/cjs/loader:1364:14)
    at Module.m._compile (/Users/antonis/git/sentry-javascript-bundler-plugins/packages/integration-tests/node_modules/ts-node/src/index.ts:1618:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1422:10)
    at Object.require.extensions.<computed> [as .ts] (/Users/antonis/git/sentry-javascript-bundler-plugins/packages/integration-tests/node_modules/ts-node/src/index.ts:1621:12) {
  status: 1,
  signal: null,
  output: [ null, null, null ],
  pid: 85058,
  stdout: null,
  stderr: null
}
ERROR: "test:setup" exited with 1.

 ——————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

 >  NX   Ran target test for project @sentry-internal/integration-tests and 6 task(s) it depends on (14s)
 
    ✖    1/7 failed
    ✔    6/7 succeeded [5 read from cache]

lucas-zimerman · 2025-08-07T10:26:43Z

packages/babel-plugin-component-annotate/test/test-plugin.test.ts

+    expect(fragmentResults.avg).toBeLessThan(300);
+  });
+
+  it("should not consume excessive memory", () => {


This may have contaminated data since it's running other tests in parallel.

Suggestion: Use spawnSync to invoke node and run this specific test.
the stdout from the test should return the memory used by the project.

I think we can never run benchmarks in test processes, there is too much risk of variance.

I would much prefer using something like https://github.com/RafaelGSS/bench-node and making it a separate CI step. We can do this in a follow-up PR though.

Sounds good. I removed benchmarks for now with 29e996f and will iterate separately. In any case I think they served as a sanity check for now.

lucas-zimerman · 2025-08-07T10:39:00Z

packages/babel-plugin-component-annotate/src/index.ts

 ): void {
  if (!jsxNode) {
    return;
  }
+
+  // Use provided componentName or fall back to context componentName
+  const currentComponentName = componentName !== undefined ? componentName : context.componentName;


Will there be any cases where componentName is null? If so, do we want to fall back to the context data instead of setting null?
If true, we can use the following suggestion:

Suggested change

const currentComponentName = componentName !== undefined ? componentName : context.componentName;

const currentComponentName = componentName ?? context.componentName;

Actually from the code below you do set componentName as null in some function calls, on that context, I believe we should give it a chance for the context.componentName to be set

Good point @lucas-zimerman 👍
Updated with ea68863

Lms24 · 2025-08-07T10:50:12Z

@Lms24 I think this is a good idea 👍 I'm facing some issues running the integration tests (unmodified) localy (see log). I'll investigate this further and iterate on this or a follow up PR. Any feedback is welcome 🙏

following up with it sounds fine to me, thanks! I just tried running them locally and for me, all 73 pass (i do get a shit ton of console logs and warnings from the plugins though). Does the one you want to adjust/modify fail? Otherwise, I'd try to ignore it as best as possible for now if you don't find out what's causing the fails 😅

You can also cd into the integration tests dir, first run yarn test:setup and then yarn test:jest -t "..." to run specific tests.

packages/babel-plugin-component-annotate/src/index.ts

lucas-zimerman · 2025-08-07T10:52:48Z

packages/babel-plugin-component-annotate/src/index.ts

+            // import { Fragment } from 'react' -> Fragment
+            // import { Fragment as F } from 'react' -> F
+            if (spec.imported.name === "Fragment") {
+              fragmentAliases.add(spec.local.name);


Will the Alias be applied globally?
Asking in case of edge cases where someone alias Fragment as F but on other page they alias something else as F

That's a good question @lucas-zimerman and @alwx 👍
Regarding same aliases in different files I think this should already be covered since babel processes files one by one.
Regarding aliases on the same source file I added a test case with 4dcfbf5
I think the correct thing to do is to not add the data-sentry-element in that case since it may result in a crash if it is a fragment.
I'll be happy to iterate if you have another case in mind 🙏

packages/babel-plugin-component-annotate/src/index.ts

lucas-zimerman · 2025-08-07T11:01:35Z

packages/babel-plugin-component-annotate/src/index.ts

+          if (spec.type === "ImportSpecifier" && spec.imported.type === "Identifier") {
+            // import { Fragment } from 'react' -> Fragment
+            // import { Fragment as F } from 'react' -> F
+            if (spec.imported.name === "Fragment") {


Another question, should we safeguard that it's also
spec.source.value === "react" so we know Fragment is actually from react and not some user code?

I think this should be covered with the outer check if (source === "react" || source === "React")

packages/babel-plugin-component-annotate/src/index.ts

Co-authored-by: LucasZF <[email protected]>

alwx · 2025-08-07T11:28:26Z

packages/babel-plugin-component-annotate/src/index.ts

-  sourceFileName: string | undefined,
-  attributeNames: string[],
-  ignoredComponents: string[]
+  componentName?: string | null


I feel slightly confused that componentName could be a string, undefined and null now — in which situations it could be null and undefined and what's the difference between these two cases?

Good point @alwx 👍
I tried to simplify this with ea68863

antonis · 2025-08-11T06:56:14Z

Thanks all for your feedback 🙇
This is now ready for another pass 🤞

Lms24

This looks good to me, thanks for making all the changes. Tests seem to cover a lot of cases, so I'm fine with merging. Let's wait until EOD for others to chime in and then

alwx

Looks good, thanks for handling that!

@grushetsky

| datasource | package | from | to | | ---------- | ------------------- | ----- | ----- | | npm | @sentry/vite-plugin | 3.6.1 | 4.1.1 | ## [v4.1.1](https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/HEAD/CHANGELOG.md#411) - fix(react-native): Enhance fragment detection for indirect references ([#767](getsentry/sentry-javascript-bundler-plugins#767)) ## [v4.1.0](https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/HEAD/CHANGELOG.md#410) - feat(deps): Bump [@sentry/cli](https://github.com/sentry/cli) to 2.51.0 [#786](getsentry/sentry-javascript-bundler-plugins#786) - feat(core): Add flag for disabling sourcemaps upload [#785](getsentry/sentry-javascript-bundler-plugins#785) - fix(debugId): Add guards for injected code to avoid errors [#783](getsentry/sentry-javascript-bundler-plugins#783) - docs(options): Improve JSDoc for options [#781](getsentry/sentry-javascript-bundler-plugins#781) - feat(core): Expose method for injecting debug Ids from plugin manager [#784](getsentry/sentry-javascript-bundler-plugins#784) ## [v4.0.2](https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/HEAD/CHANGELOG.md#402) - fix(core): Make `moduleMetadata` injection snippet ES5-compliant ([#774](getsentry/sentry-javascript-bundler-plugins#774)) ## [v4.0.1](https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/HEAD/CHANGELOG.md#401) - fix(core): Make plugin inject ES5-friendly code ([#770](getsentry/sentry-javascript-bundler-plugins#770)) - fix(core): Use `renderChunk` for release injection for Rollup/Rolldown/Vite ([#761](getsentry/sentry-javascript-bundler-plugins#761)) Work in this release was contributed by [@grushetsky](https://github.com/grushetsky). Thank you for your contribution! ## [v4.0.0](https://github.com/getsentry/sentry-javascript-bundler-plugins/blob/HEAD/CHANGELOG.md#400) ##### Breaking Changes - (Type change) Vite plugin now returns `VitePlugin` type instead of `any` - Deprecated function `getBuildInformation` has been removed ##### List of Changes - feat(core)!: Remove `getBuildInformation` export ([#765](getsentry/sentry-javascript-bundler-plugins#765)) - feat(vite)!: Update return type of vite plugin ([#728](getsentry/sentry-javascript-bundler-plugins#728))

fix(react-native): Enhance fragment detection for indirect references

3070c36

kahest requested review from Lms24, AbhiPrasad and chargome July 21, 2025 08:22

AbhiPrasad reviewed Jul 21, 2025

View reviewed changes

Lms24 reviewed Jul 21, 2025

View reviewed changes

antonis added 7 commits August 6, 2025 13:23

Use strongly typed sentryFragmentContext

b44fcc8

Fixes lint issues

034a818

Merge branch 'main' into antonis/enhanced-fragment-detection

1ee4667

Fix tests

d208ae1

Refactor the long parameter lists to use a context object

d728ba5

Update snapshot

00b9834

Adds performance benchmarks

4063095

antonis marked this pull request as ready for review August 7, 2025 08:01

antonis requested review from Lms24 and AbhiPrasad August 7, 2025 08:05

lucas-zimerman reviewed Aug 7, 2025

View reviewed changes

packages/babel-plugin-component-annotate/src/index.ts Outdated Show resolved Hide resolved

lucas-zimerman reviewed Aug 7, 2025

View reviewed changes

packages/babel-plugin-component-annotate/src/index.ts Outdated Show resolved Hide resolved

lucas-zimerman reviewed Aug 7, 2025

View reviewed changes

packages/babel-plugin-component-annotate/src/index.ts Outdated Show resolved Hide resolved

antonis and others added 3 commits August 7, 2025 14:11

Update comment for clarity

5389cb4

Co-authored-by: LucasZF <[email protected]>

Simplify conditional logic

d150d93

Co-authored-by: LucasZF <[email protected]>

Fix formatting

016ec87

antonis added 3 commits August 7, 2025 14:16

Adjust benchmark baseline after CI run

0fa60c5

fix typecheck issue

039e414

Make benchmark less flaky

5961ffe

alwx reviewed Aug 7, 2025

View reviewed changes

antonis added 6 commits August 7, 2025 14:31

Check fragment sooner

314f7bc

Simplify componentName null/undefined handling

ea68863

Add same alias test

4dcfbf5

Removes benchmarks from unit tests

29e996f

Merge branch 'main' into antonis/enhanced-fragment-detection

485a65e

Add changelog

5e866c3

antonis requested review from alwx and lucas-zimerman August 7, 2025 15:26

antonis added 2 commits August 7, 2025 18:32

Remove unused function after benchmarks removal

f83899e

Merge branch 'main' into antonis/enhanced-fragment-detection

9c78d93

Merge branch 'main' into antonis/enhanced-fragment-detection

cb249f8

Lms24 approved these changes Aug 12, 2025

View reviewed changes

alwx approved these changes Aug 12, 2025

View reviewed changes

AbhiPrasad approved these changes Aug 12, 2025

View reviewed changes

Lms24 merged commit 1d1b4a0 into getsentry:main Aug 12, 2025
37 of 40 checks passed

Lms24 mentioned this pull request Aug 12, 2025

meta: Changelog entry for 4.1.1 #790

Merged

antonis mentioned this pull request Aug 13, 2025

chore(deps): update Bundler Plugins to v4.1.1 getsentry/sentry-react-native#5072

Merged

	const currentComponentName = componentName !== undefined ? componentName : context.componentName;
	const currentComponentName = componentName ?? context.componentName;

fix(react-native): Enhance fragment detection for indirect references #767

fix(react-native): Enhance fragment detection for indirect references #767

Conversation

antonis commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AbhiPrasad left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lms24 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antonis commented Jul 21, 2025

Uh oh!

antonis commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lms24 commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antonis Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alwx Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antonis commented Aug 11, 2025

Uh oh!

Lms24 left a comment

Choose a reason for hiding this comment

Uh oh!

alwx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

antonis commented Jul 18, 2025 •

edited

Loading

Lms24 left a comment •

edited

Loading

antonis commented Aug 7, 2025 •

edited

Loading

Lms24 commented Aug 7, 2025 •

edited

Loading

antonis Aug 7, 2025 •

edited

Loading

alwx Aug 7, 2025 •

edited

Loading